Skip to content

Add multi-repo workspace support#426

Merged
wesm merged 12 commits intoroborev-dev:mainfrom
darrenhaas:workspace-support
Mar 5, 2026
Merged

Add multi-repo workspace support#426
wesm merged 12 commits intoroborev-dev:mainfrom
darrenhaas:workspace-support

Conversation

@darrenhaas
Copy link
Contributor

@darrenhaas darrenhaas commented Mar 5, 2026

Summary

Adds workspace mode: roborev list and roborev review now work from non-git parent directories that contain multiple git repos. Running roborev list from such a directory filters jobs/repos by path prefix, showing results across all child repositories.

Changes

Workspace detection (cmd/roborev/list.go, cmd/roborev/review.go)

  • roborev list detects when the working directory is not a git repo and no --repo is specified — switches to workspace mode, sending the current directory as a repo_prefix query param
  • roborev review from a non-git parent directory lists child repos containing .git (file or directory) and prints hint paths with full absolute paths
  • Auto-detected branch is suppressed in workspace mode (filtering one repo's branch across a workspace is meaningless), but explicit --branch flags are always honored via cmd.Flags().Changed("branch")

Composable repo query options (internal/storage/repos.go)

Replaced three near-identical functions (ListReposWithReviewCounts, ListReposWithReviewCountsByPrefix, ListReposWithReviewCountsByBranch) with a single function using composable options:

  • WithRepoPathPrefix(prefix) — filter repos by path prefix
  • WithRepoBranch(branch) — filter repos by branch (switches from LEFT JOIN to INNER JOIN + HAVING)
  • Both options can be combined, enabling prefix+branch filtering that was previously impossible

SQL ESCAPE character fix (internal/storage/jobs.go, internal/storage/repos.go)

Changed escapeLike from \ to ! as the ESCAPE character. The backslash conflicted with Windows path separators stored in root_path, causing invalid escape sequences in LIKE clauses.

Cross-platform path normalization

  • GetOrCreateRepo and GetRepoByPath apply filepath.ToSlash at write time so DB rows always store forward-slash paths regardless of platform
  • RenameRepo and FindRepo normalize paths before querying
  • Server handlers apply filepath.ToSlash(filepath.Clean(...)) to prefix query params, normalizing trailing slashes, .. segments, and backslashes
  • Client-side (list.go) normalizes the workspace prefix to forward slashes before sending

Git worktree detection (cmd/roborev/review.go)

findChildGitRepos now detects .git as either a file (worktrees, submodules) or a directory (regular repos). Previously only checked IsDir().

Path normalization in server handlers (internal/daemon/server.go)

handleListJobs and handleListRepos apply filepath.Clean + filepath.ToSlash to prefix params, and pass both prefix and branch options when present (previously mutually exclusive via if/else chain).

🤖 Generated with Claude Code

@darrenhaas darrenhaas closed this Mar 5, 2026
@darrenhaas darrenhaas deleted the workspace-support branch March 5, 2026 00:10
@darrenhaas darrenhaas restored the workspace-support branch March 5, 2026 00:10
@darrenhaas darrenhaas reopened this Mar 5, 2026
darrenhaas and others added 9 commits March 4, 2026 21:16
When running roborev from a parent directory containing multiple git repos
(e.g., a monorepo workspace), commands now gracefully handle not being in
a git repo: `list` shows jobs across all child repos via repo_prefix filter,
`show` suggests using job IDs, and `review` lists available child repos
with --repo hints. Includes LIKE wildcard escaping for path safety.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change escapeLike to use '!' instead of '\' as the ESCAPE character
in LIKE clauses. The backslash escape conflicted with Windows path
separators stored in root_path, causing prefix filters to fail on
Windows. Update all three LIKE ESCAPE clauses (ListJobs, CountJobStats,
ListReposWithReviewCountsByPrefix).

Also fix review hint paths to show full absolute paths instead of bare
repo names, and use strings.Builder to satisfy the modernize linter.

Add TestEscapeLike and TestPrefixFilterWithSpecialChars to verify
correct escaping of !, %, and _ characters.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace three near-identical ListReposWithReviewCounts* functions with
a single ListReposWithReviewCounts(opts ...ListReposOption) using
composable WithRepoPathPrefix and WithRepoBranch options. This matches
the existing ListJobsOption pattern in jobs.go and enables combined
prefix+branch filtering that was previously impossible (the if/else
chain in handleListRepos made them mutually exclusive).

Add filepath.Clean normalization for repo_prefix and prefix query
params in handleListJobs and handleListRepos to handle trailing
slashes and ".." segments.

Add tests for combined prefix+branch filtering, trailing-slash
normalization, and dot-dot path normalization.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When running from a non-git parent directory (workspace mode), the
auto-detected branch was correctly suppressed since it makes no sense
to filter by one repo's branch across a workspace. However, an
explicit --branch flag was also being dropped. Check cmd.Flags().Changed
to honor explicitly passed --branch values in workspace mode.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
findChildGitRepos checked info.IsDir() for .git, which missed git
worktrees and submodules where .git is a file pointing to the actual
git directory. Remove the IsDir() check — os.Stat succeeding is
sufficient to confirm the entry is a git repo.

The hint path fix (showing full absolute paths instead of bare repo
names) was included in the earlier ESCAPE commit since it touched the
same code. This commit adds the .git file detection fix and tests for
both behaviors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
#7920: Add CountJobStats and ListReposWithReviewCounts prefix tests
with special chars (!, %, _) and backslash-in-path test verifying no
SQL errors after the ESCAPE char change.

#7922: Add dot-dot normalization test for /api/repos prefix endpoint.

#7923: Add workspace-mode integration tests verifying auto-detected
branch is suppressed and explicit --branch is honored.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Complete seeded jobs before asserting CountJobStats so the test
validates actual prefix-filter behavior (2 done under prefix, not 3)
instead of vacuously passing with all-zero stats from queued jobs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add filepath.ToSlash after filepath.Clean on repo_prefix and prefix
query params in handleListJobs and handleListRepos, and on the
client-side prefix in list.go. The SQL LIKE clause uses '/%' as the
separator, so backslash-separated Windows paths from filepath.Abs or
filepath.Clean would fail to match.

Add tests for roborev show outside a git repo: default invocation
returns a guidance error, explicit --job ID still works.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add TestHandleListJobsSlashNormalization and
TestHandleListReposSlashNormalization verifying that forward-slash
prefixes (the output of filepath.ToSlash) correctly match stored
repo paths through the handler pipeline.

Note: filepath.ToSlash is already platform-conditional by design —
it replaces os.PathSeparator with '/', which is a no-op on POSIX
(where os.PathSeparator is already '/'). Backslashes in POSIX path
segments are valid filename characters and are not transformed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wesm wesm force-pushed the workspace-support branch from bac4a9a to cf7bd80 Compare March 5, 2026 03:16
@roborev-ci
Copy link

roborev-ci bot commented Mar 5, 2026

roborev: Combined Review (cf7bd80)

Verdict: The PR safely implements workspace prefix filtering, but introduces two Medium severity bugs related to root path handling and Windows path
normalization that should be addressed.

Medium

1. Prefix filter fails at filesystem/drive roots due to hardcoded "/%" concatenation

  • Files: [internal/storage/jobs.go:1424](/home/roborev/repos/roborev/internal/storage/jobs
    .go:1424), internal/storage/jobs.go:1595, [internal/storage/repos.go:160](/home/robore
    v/repos/roborev/internal/storage/repos.go:160), internal/daemon/server.go:821, internal/daemon/server.
    go:928
  • Details: If prefix is / (or C:/), the SQL pattern becomes //% (or C://%), which
    won’t match normal child paths. This breaks workspace filtering when run from root directories.
  • Suggested Fix: Build a full pattern in Go with root-aware handling (special-case root, otherwise prefix + "/%") instead of SQL ? || '/%'.

2. Likely Windows regression: forward-
slash normalized prefixes may not match backslash-stored repo paths

  • Files: internal/daemon/server.go:821, internal/daemon/server.go:
    928
    , [internal/storage/jobs.go:1424](/home/roborev/repos/roborev/internal/storage/jobs.go:14
    24), internal/storage/repos.go:160
  • Details: Prefixes are normalized to /, but matching is against r.root_path as
    stored. If DB rows are native Windows \ paths, LIKE 'C:/.../%' will miss them.
  • Suggested Fix: Normalize root_path at write time (recommended), or normalize in query (REPLACE(r.root_path, '\\', '/')), then match.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

wesm and others added 2 commits March 4, 2026 21:22
GetOrCreateRepo and GetRepoByPath now apply filepath.ToSlash after
filepath.Abs so root_path is always stored with forward slashes,
matching the LIKE '/%' pattern used by prefix filters. This fixes
workspace prefix filtering on Windows where filepath.Abs returns
backslash-separated paths.

Strip trailing slashes from prefix options (WithRepoPrefix,
WithRepoPathPrefix) so root-prefix "/" doesn't produce the
double-slash LIKE pattern "//%". A root prefix trims to empty string,
disabling the filter (all repos match), which is correct.

Strengthen slash normalization test assertions to verify repo
identities (names, paths) not just counts. Add TestRootPrefixMatchesAllRepos
covering the "/" edge case.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
RenameRepo was missing filepath.ToSlash after filepath.Abs, so on
Windows the backslash path didn't match the forward-slash stored path.

Tests used hardcoded Unix paths (/tmp/workspace, /ws, etc.) which on
Windows get a drive prefix from filepath.Abs (C:/tmp/workspace),
causing prefix filters to miss. Switch to t.TempDir()-based paths
that are absolute on all platforms.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Mar 5, 2026

roborev: Combined Review (34f010d)

Verdict: Adds workspace-aware repo-prefix filtering and cross-platform path normalization, but requires a migration strategy for legacy Windows paths to prevent orphaned database records.

Medium

1. Legacy Windows DB
paths can be orphaned after slash-normalization change

  • Files: internal/storage/repos.go:18, internal/storage/repos.go:95, internal/storage/repos.go:295
  • GetOrCreateRepo, GetRepoByPath , and RenameRepo now normalize to forward slashes, but there is no migration/compat layer for existing repos.root_path rows that were previously stored with backslashes on Windows. This can create duplicate repo records and split old vs new job history.
  • Suggested fix: Add a
    migration (or temporary dual-lookup) to canonicalize old \ paths to / and merge duplicates safely (including re-pointing review_jobs.repo_id).

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Normalize repo path params to forward slashes in handleListJobs and
handleListBranches, matching the forward-slash paths stored by
GetOrCreateRepo.

Fix CI poller test harness to use repo.RootPath (normalized) instead
of raw t.TempDir() path. Fix test assertions that compared
forward-slash DB paths against native backslash paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Mar 5, 2026

roborev: Combined Review (766db4b)

Verdict: The changes successfully implement workspace-aware listing and path normalization, but introduce a High severity backward-compatibility issue for Windows deployments that requires a database migration.

High

File: [internal/storage/repos.go](/home/roborev/repos/roborev/
internal/storage/repos.go) (GetOrCreateRepo, GetRepoByPath, RenameRepo)
Problem: root_path is now normalized to forward slashes at read/write boundaries, but there is no migration or backward-compatibility path for existing Windows database rows stored with back
slashes.
Impact: Existing repositories may no longer be found by path. GetOrCreateRepo can insert duplicate repository rows for the same checkout, causing history and statistics to split across old and new repository IDs.
Suggested fix: Add a database migration (or fallback lookup) to normalize legacy repos. root_path values (backslash to slash) before or while using the new lookup behavior, and handle deduplication collisions safely.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@wesm
Copy link
Collaborator

wesm commented Mar 5, 2026

Re: review 766db4b (migration finding) — invalid, same as #7936. The workspace-support branch introduces the forward-slash normalization alongside the workspace feature itself. There are no existing DB rows with backslash root_path values from this feature because it hasn't shipped yet. No migration needed.

@wesm wesm merged commit a5457fd into roborev-dev:main Mar 5, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants